Skip to content

feat: add C implementation for stats/base/dists/lognormal/cdf#10883

Open
rautelaKamal wants to merge 1 commit intostdlib-js:developfrom
rautelaKamal:feat/stats/base/dists/lognormal/cdf-c-port
Open

feat: add C implementation for stats/base/dists/lognormal/cdf#10883
rautelaKamal wants to merge 1 commit intostdlib-js:developfrom
rautelaKamal:feat/stats/base/dists/lognormal/cdf-c-port

Conversation

@rautelaKamal
Copy link
Copy Markdown
Contributor

@rautelaKamal rautelaKamal commented Mar 11, 2026

Description

What is the purpose of this pull request?

This pull request:

  • Adds a C implementation for evaluating the cumulative distribution function (CDF) of a lognormal distribution (@stdlib/stats/base/dists/lognormal/cdf). The implementation delegates to the native stdlib_base_dists_normal_cdf function after log-transforming the input x, adhering to the modular C core approach.

Related Issues

Does this pull request have any related issues?

This pull request has the following related issues:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

AI Assistance Disclosure

  • Yes
  • No

If yes, how was AI used?
I used an AI to speed up the translation of JS tests to native tests using the Julia fixtures, and to stub out the required C files (addon.c, main.c, build configs). The generated code was then verified against project style guidelines.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

@stdlib-bot stdlib-bot added Statistics Issue or pull request related to statistical functionality. Needs Review A pull request which needs code review. Good First PR A pull request resolving a Good First Issue. labels Mar 11, 2026
@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented Mar 11, 2026

Hello! 👋

We've noticed that you've been opening a number of PRs addressing good first issues. Thank you for your interest and enthusiasm!

Now that you've made a few contributions, we suggest no longer working on good first issues. Instead, we encourage you to prioritize cleaning up any PRs which have yet to be merged and then proceed to work on more involved tasks.

Not only does this ensure that other new contributors can work on things and get ramped up on all things stdlib, it also ensures that you can spend your time on more challenging problems. 🚀

For ideas for future PRs, feel free to search the codebase for TODOs and FIXMEs and be sure to check out other open issues on the issue tracker. Cheers!

@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented Mar 11, 2026 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/lognormal/cdf $\color{green}276/276$
$\color{green}+0.00%$ 		$\color{green}23/23$
$\color{green}+0.00%$ 		$\color{green}4/4$
$\color{green}+0.00%$ 		$\color{green}276/276$
$\color{green}+0.00%$

The above coverage report was generated for the changes in this PR.

@rautelaKamal rautelaKamal marked this pull request as draft March 12, 2026 00:42
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Mar 12, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from db99cb5 to 31dab56 Compare March 12, 2026 11:06
@rautelaKamal rautelaKamal marked this pull request as ready for review March 12, 2026 15:47
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Mar 12, 2026
This was referenced Mar 30, 2026
Closed
Open
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from b5f205d to 8fd6468 Compare April 1, 2026 11:13
@github-actions github-actions bot mentioned this pull request Apr 1, 2026
Open
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 8fd6468 to 58353a4 Compare April 4, 2026 07:46
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 58353a4 to 82c5d79 Compare April 12, 2026 07:01
kgryte
kgryte reviewed Apr 12, 2026
View reviewed changes
lib/node_modules/@stdlib/stats/base/dists/lognormal/cdf/benchmark/c/benchmark.c
#define NAME "lognormal-cdf"
#define ITERATIONS 1000000
#define REPEATS 3

Copy link
Copy Markdown
Member

@kgryte kgryte Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing all the Doxygen comments. https://github.com/stdlib-js/stdlib/blob/develop/tools/snippets/benchmark/c/benchmark.c

Copy link
Copy Markdown
Contributor Author

@rautelaKamal rautelaKamal Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've addressed the feedback in the latest push (added Doxygen comments, migrated to the new RNG, and updated isAlmostSameValue usage). This PR should be ready for re-review.

kgryte
kgryte reviewed Apr 12, 2026
View reviewed changes
lib/node_modules/@stdlib/stats/base/dists/lognormal/cdf/test/test.native.js Outdated
t.strictEqual( y, expected[i], 'x: '+x[i]+', mu:'+mu[i]+', sigma: '+sigma[i]+', y: '+y+', expected: '+expected[i] );
} else {
delta = abs( y - expected[ i ] );
tol = 1500.0 * EPS * abs( expected[ i ] );
Copy link
Copy Markdown
Member

@kgryte kgryte Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should go ahead and migrate to using isAlmostSameValue for ULP-based difference testing.

@kgryte kgryte changed the title feat: add C implementation for @stdlib/stats/base/dists/lognormal/cdf feat: add C implementation for stats/base/dists/lognormal/cdf Apr 12, 2026
@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Apr 12, 2026
@kgryte
Copy link
Copy Markdown
Member

kgryte commented Apr 12, 2026

@rautelaKamal I updated your PR title. Would you mind updating your other PR titles similarly? This reduces the amount of busy work required from maintainers. Cheers!

@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 82c5d79 to 8b839ee Compare April 13, 2026 07:06
@rautelaKamal rautelaKamal changed the title feat: add C implementation for stats/base/dists/lognormal/cdf feat: add C implementation for stats/base/dists/lognormal/cdf Apr 13, 2026
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 8b839ee to 5b610f2 Compare April 13, 2026 07:49
@rautelaKamal rautelaKamal force-pushed the feat/stats/base/dists/lognormal/cdf-c-port branch from 5b610f2 to f2b9171 Compare April 13, 2026 08:25
@rautelaKamal
Copy link
Copy Markdown
Contributor Author

rautelaKamal commented Apr 14, 2026

I have updated the PR titles as requested and addressed the feedback regarding isAlmostSameValue, Doxygen, and RNG migration. This PR is now ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kgryte kgryte kgryte left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Good First PR A pull request resolving a Good First Issue. Needs Changes Pull request which needs changes before being merged. Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rautelaKamal @stdlib-bot @kgryte